-
-
Notifications
You must be signed in to change notification settings - Fork 827
Implement the "General" tab of new user settings #2491
Conversation
Makes the flair options in old settings look broken (cosmetic issues), but it's fine because we're ripping that out in due time.
This also changes the layout slightly in the user settings, but nothing detrimental.
The tab component is getting a bit hard to navigate
Also bring in the compact timeline option. Without minor CSS changes, the old user settings are completely unusable with this change. As such, minimal effort has been put in to have it be useful. Similarly, the changes drop the use of radio groups and the old theme selector was the only one that used it. See the comments for more details on how/why this was mitigated the way it was.
Looks like the build is angry about branches again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I think it looks great. 😁
I'd like to understand when we choose to import
vs. sdk.getComponent
first before approving, though.
@@ -18,6 +18,7 @@ import React from "react"; | |||
import PropTypes from 'prop-types'; | |||
import SettingsStore from "../../../settings/SettingsStore"; | |||
import { _t } from '../../../languageHandler'; | |||
import ToggleSwitch from "./ToggleSwitch"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we choose when to import vs. call sdk.getComponent
? It seems arbitrary...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea. The guideline I use is sdk.getComponent
when copying old code in and import
whenever I can. Usually the stuff brought in by sdk,getComponent
is old and not easily import
-able too.
Edit: have added it to our retro topics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I also asked in #riot-dev as well.
@jryans please take another look. I've also pushed a temporary branch to riot-web for the satisfaction of green checkmarks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your replies and updates. Looks good to proceed. 😁
Fixes element-hq/element-web#8200
Part of element-hq/element-web#7605
This PR does a number of things to support the new tab:
DragDropContext
around the flair settings which is needed to make react not-crash. This is scheduled for removal in Redesign: Remaining bits after both user and room settings are implemented element-hq/element-web#8207.Field
to support reading the value without controlling the inputToggleSwitch
SettingsFlag
over to usingToggleSwitch
UserSettings
have been visibly broken but still work. Minor cosmetic fixes were done to keep it moderately usable, however it is expected that it doesn't look like what it used to.